Fix #78374: Warning when closing FTPS connection after FTP_PUT #7362
Conversation
`ftp_ssl_shutdown()` attempts a bidirectional shutdown handshake, i.e. we're sending a close_notify alert, and then wait for the peer to send its close_notify alert. If the peer doesn't sent it, typically because it already closed the connection, we should not raise a warning, but rather ignore this condition.
|
New test fails on azure. |
The latter might not be supported by the OpenSSL library.
|
Thanks! I've pushed a potential fix. |
|
Nope, that wasn't the issue. This may get funny. |
|
Apparently, the test fails only with OpenSSL 1.1, but passes with OpenSSL 1.0. I have no idea why, though. Any hints appreciated! |
|
From the comments above, I cannot conclude what the findings are with regard to the problem. I still get the warning. Apache/2.4.57 (Win64) OpenSSL/1.1.1t PHP/8.3.17. OpenSSL Library Version: OpenSSL 3.0.15 3 Sep 2024. Is this pull request closed, because there is no solution (yet)? Or is there a newer PHP version with a fix? I can't find anything in the PHP version history. Any help would be appreciated. Thanks! |
|
Indeed, this bug has still not been fixed; I had given up on it.
Which OpenSSL version is it, 1.1.1t or 3.0.15? |
|
Sorry for being ambiguous. |
|
In C:\Program Files\Apache Software Foundation\Apache_2.4.57_VS16\bin I have libssl-1_1-x64.dll. (Version 1.1.1t). This file came with Apache. (All my FTP functions work so far, except for this warning.) |
|
I have been checking and thinking about this one. I think that waiting for the peer to close the connection and error otherwise is actually the right thing to do in general and this is also what OpenSSL does by default. The reason for that is that it might allow the truncation attack. However in the FTP context, the truncation attack should be less likely because it awaits the responses and the data that is read is discarded anyway. I guess we could just call SSL_shutdown twice which should actually handle all of this. It would still error so we would need to set SSL_OP_IGNORE_UNEXPECTED_EOF which is something that this should do anyway because it could in some case have some data to read (e.g. session ticket record) but then close the connection. The only potentially issue with second SSL_shutdown would be that if server kept connection open, it wouldn't timed out so maybe this approach is safer. I think those changes makes sense. As mentioned it should also set SSL_OP_IGNORE_UNEXPECTED_EOF to ctx options when created but that's more edge case. The thing that needs to improve is the test though. I don't think this is going to close the connection so it seems not like the test for this. I'm just working on custom TLS 1.3 PHP implementation (as part of other work to catch some other cases) that would allow to recreate this so will try to look into this then. |
4 participants
ftp_ssl_shutdown()attempts a bidirectional shutdown handshake, i.e.we're sending a close_notify alert, and then wait for the peer to send
its close_notify alert. If the peer doesn't sent it, typically because
it already closed the connection, we should not raise a warning, but
rather ignore this condition.
Note that the test case is somewhat bogus as it wouldn't fail without the changes to ftp.c; I wouldn't know how to accomplish that. Still, the test case makes some sense on its own, since it tests uploading via FTPS.